Use config file instead of editing source code by Leonidas-from-XIV · Pull Request #210 · ocaml-dune/binary-distribution

Leonidas-from-XIV · 2026-03-03T16:13:16Z

This PR changes the way Sandworm is reconfigured by moving from editing sourcecode as part of an GH workflow into using a config file.

Some notes about it:

It uses TOML. No particular significance about the format. The OCaml implementation seems to be well-maintained and the format is reasonably accepted. Could have used INI files as well, but the OCaml implementation is less mature.
This PR gives up on the "staging" branch setup. It is easier to have a single main branch and have test as an option to its execution. We need to rework how to test things in either case.
The config options are taken from the workflow

Closes #199

Signed-off-by: Marek Kubica <marek@tarides.com>

The values are taken from the nightly workflow. Signed-off-by: Marek Kubica <marek@tarides.com>

Signed-off-by: Marek Kubica <marek@tarides.com>

README.md

Signed-off-by: Marek Kubica <marek@tarides.com>

Alizter · 2026-03-06T14:47:05Z

Looks ok to me. It might be worth re-asking: Do we really want to get rid of staging? Isn't the point to isolate from "prod"?

Leonidas-from-XIV · 2026-03-06T15:18:24Z

I think we should have a way to test this, but the way this is currently setup is a mess:

The config is in the workflows where it gets modified based on some conditions. This is not great for auditing and understanding what happens where and how.
Setting up staging is quite complicated as this requires to re-set the branch every time and do the testing. Its currently 268 commits behind main and only one person can ever test at once.
The metadata.json is part of the repo, thus one needs to be careful about not accidentally committing the bundle to the wrong branch otherwise the server serves redirects to non-existing files.
The metadata.json is also part of the Docker container, so it needs to be redeployed on every change

What I think would be better is to have two environments with completely separate metadata.json that are not part of the repo (since metadata.json is more like a database than source code), with the configuration in the repository. Thus e.g. the workflows can choose which environment to upload to and the web server would be reading their own separate metadata.json from somewhere to know what they need to serve.

This is however a definitely out of the scope of this PR (and possibly more work than building a better design from scratch). So I can try to make this work with the staging branch but I am unsure whether it's worth the added work. @shonfeder what do you think?

Signed-off-by: Marek Kubica <marek@tarides.com>

shonfeder · 2026-03-06T20:31:23Z

dune-project

  dream
  dream-encoding
  tailwindcss
+  (toml (>= 7))


If we need a runtime loadable config file, why not just use JSON? We already have yojson as a dep so it saves a dependency, we are already using jsonh and it avoids the need introduce yet another config format into the code base.

But, see my following comments, as I don't understand why we need a runtime loadable config file.

shonfeder · 2026-03-06T21:01:02Z

.github/workflows/binary-stable.yaml

+      - name: Export executables and generate HTML (test)
+        if: ${{ inputs.test }}
+        run: ./_build/install/default/bin/sandworm sync --commit "${{ needs.check_build.outputs.commit }}" --tag "${{ needs.check_build.outputs.release }}" --config config-test.toml
+
+      - name: Export executables and generate HTML (production)
+        if: ${{ !inputs.test }}


Why do we need run-time loadable configuration files at all? Couldn't we just have a CLI flag that selected which of two records to use (or which of two modules, using the current approach).

shonfeder · 2026-03-06T21:02:27Z

bin/config.ml

-module Server = struct
-  let bucket_dir = "/dune/"
-  let rclone_bucket_ref = Format.sprintf "dune-binary-distribution:%s" bucket_dir
-  let url = "https://get.dune.build/"
+module type FILE = sig
+  val file_name : string
 end

-module Path = struct
-  let artifacts_dir = "./artifacts"
-  let metadata = "./metadata.json"
-  let rclone = "./rclone.conf"
-  let install = "./static/install"


Following on my previous question, why not just keep the data in OCaml and have a CLI flag to pick which of two different configs to us? Then we avoid needing any deserialization or runtime config loading.

shonfeder · 2026-03-06T21:05:05Z

.github/workflows/binary-nightly.yaml

-      - name: Export executables and generate html
-        shell: bash
+      - name: Export executables and generate HTML (test)
+        if: ${{ github.ref == 'refs/heads/staging' }}


The PR description says we are getting rid the "staging" branch? But looks not?

shonfeder · 2026-03-06T21:11:22Z

What I think would be better is to have two environments with completely separate metadata.json that are not part of the repo (since metadata.json is more like a database than source code

I don't see a problem with keeping "the database" in the source code for a project like this. It give us an easy way to rollback, and keeps the infra minimal. Could we address your concerns with a metadata-test.json file?

This is however a definitely out of the scope of this PR (and possibly more work than building a better design from scratch). So I can try to make this work with the staging branch but I am unsure whether it's worth the added work. @shonfeder what do you think?

IMO, it is important that we can test changes before merging them into the trunk. But that doesn't necessarily mean we need a fixed staging branch: rather a way of deploying a selected branch to the staging environment. Does this make sense?

shonfeder · 2026-03-06T21:25:13Z

.github/workflows/binary-nightly.yaml

+        if: ${{ github.ref == 'refs/heads/staging' }}
+        run: ./_build/install/default/bin/sandworm sync --commit "${{ needs.binary.outputs.git-commit }}" --config config-test.toml
+
+      - name: Export executables and generate HTML (production)
+        if: ${{ github.ref != 'refs/heads/staging' }}


Should this rather require that the branch is main?

shonfeder · 2026-03-06T21:26:33Z

.github/workflows/binary-nightly.yaml


-      - name: Export executables and generate html
-        shell: bash
+      - name: Export executables and generate HTML (test)


The test/prod distinction seems the most important thing here. So maybe a more evident naming like this?

Suggested change

- name: Export executables and generate HTML (test)

- name: 'TEST -- Export executables and generate HTML'

Leonidas-from-XIV force-pushed the toml-config branch 4 times, most recently from c7c4698 to 4db6a38 Compare March 5, 2026 13:54

Leonidas-from-XIV added 7 commits March 6, 2026 15:07

Add toml dependency

08468f1

Signed-off-by: Marek Kubica <marek@tarides.com>

Implement TOML parsing

be63199

Signed-off-by: Marek Kubica <marek@tarides.com>

Add test configuration

a819062

The values are taken from the nightly workflow. Signed-off-by: Marek Kubica <marek@tarides.com>

Use test config if it is a test run

14481ae

Signed-off-by: Marek Kubica <marek@tarides.com>

Update documentation to match TOML config

9fea36b

Signed-off-by: Marek Kubica <marek@tarides.com>

Add configuration to Docker container

701bc5e

Signed-off-by: Marek Kubica <marek@tarides.com>

Update documentation to align more with reality

ef99b87

Signed-off-by: Marek Kubica <marek@tarides.com>

Leonidas-from-XIV force-pushed the toml-config branch from 4db6a38 to ef99b87 Compare March 6, 2026 14:07

Leonidas-from-XIV marked this pull request as ready for review March 6, 2026 14:18

Leonidas-from-XIV requested review from Alizter, Sudha247 and shonfeder March 6, 2026 14:18

Alizter reviewed Mar 6, 2026

View reviewed changes

README.md Outdated Show resolved Hide resolved

Fix markup

6197560

Signed-off-by: Marek Kubica <marek@tarides.com>

Leonidas-from-XIV requested a review from Alizter March 6, 2026 14:26

Alizter approved these changes Mar 6, 2026

View reviewed changes

Leonidas-from-XIV added 2 commits March 6, 2026 16:28

Reintroduce staging instead of test variable

7c24795

Signed-off-by: Marek Kubica <marek@tarides.com>

Add instructions how to deploy the right config to staging branch

c2822a3

Signed-off-by: Marek Kubica <marek@tarides.com>

Leonidas-from-XIV force-pushed the toml-config branch from 7bcebf5 to c2822a3 Compare March 6, 2026 15:38

shonfeder reviewed Mar 6, 2026

View reviewed changes

shonfeder requested changes Mar 6, 2026

View reviewed changes

shonfeder reviewed Mar 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use config file instead of editing source code#210

Use config file instead of editing source code#210
Leonidas-from-XIV wants to merge 10 commits intoocaml-dune:mainfrom
Leonidas-from-XIV:toml-config

Leonidas-from-XIV commented Mar 3, 2026 •

edited

Loading

Uh oh!

Uh oh!

Alizter commented Mar 6, 2026

Uh oh!

Leonidas-from-XIV commented Mar 6, 2026

Uh oh!

shonfeder Mar 6, 2026 •

edited

Loading

Uh oh!

shonfeder Mar 6, 2026

Uh oh!

shonfeder Mar 6, 2026

Uh oh!

shonfeder Mar 6, 2026

Uh oh!

shonfeder commented Mar 6, 2026 •

edited

Loading

Uh oh!

shonfeder Mar 6, 2026

Uh oh!

shonfeder Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	- name: Export executables and generate HTML (test)
	- name: 'TEST -- Export executables and generate HTML'

Conversation

Leonidas-from-XIV commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Alizter commented Mar 6, 2026

Uh oh!

Leonidas-from-XIV commented Mar 6, 2026

Uh oh!

shonfeder Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shonfeder Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

shonfeder Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

shonfeder Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

shonfeder commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shonfeder Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

shonfeder Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Leonidas-from-XIV commented Mar 3, 2026 •

edited

Loading

shonfeder Mar 6, 2026 •

edited

Loading

shonfeder commented Mar 6, 2026 •

edited

Loading